Skip to content

Conversation

@prdoyle
Copy link
Contributor

@prdoyle prdoyle commented Feb 5, 2025

Problem

The current errors and logs don't give an indication of which plugin (or server or agent) is requesting the entitlement, and so it's hard to know what needs fixing.

Also, instead of the requestingClass, we sometimes report the callerClass, which is not useful or relevant.

Fix

In the memoized ModuleEntitlements record, include the name of the "component". This is either the plugin name, or else a special string like (server) or (agent).

Clean up the exceptions and logging to report the component, module, and class information in a consistent order and format.

This lets us produce error messages that guide the user to add the right
entitlement to the right plugin/server/etc.
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Feb 5, 2025
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

public static final ModuleEntitlements NONE = new ModuleEntitlements(Map.of(), FileAccessTree.EMPTY);
public static final String UNKNOWN_COMPONENT_NAME = "(unknown)";
public static final String SERVER_COMPONENT_NAME = "(server)";
public static final String AGENT_COMPONENT_NAME = "(agent)";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually apm agent. Just "agent" may be confused with the entitlement agent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point.


record ModuleEntitlements(Map<Class<? extends Entitlement>, List<Entitlement>> entitlementsByType, FileAccessTree fileAccess) {
public static final ModuleEntitlements NONE = new ModuleEntitlements(Map.of(), FileAccessTree.EMPTY);
public static final String UNKNOWN_COMPONENT_NAME = "(unknown)";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can all be at most package private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh true. I think they're public from a prior revision of this code.

@prdoyle prdoyle enabled auto-merge (squash) February 5, 2025 22:52
@prdoyle prdoyle merged commit ba343c1 into elastic:main Feb 5, 2025
21 checks passed
prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request Feb 6, 2025
* Report componentName in ModuleEntitlements.

This lets us produce error messages that guide the user to add the right
entitlement to the right plugin/server/etc.

* Include component names in errors and logs

* Name APM agent specifically.

Avoids confusion with the entitlements agent.

* Entitlement component names package private

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request Feb 6, 2025
* Report componentName in ModuleEntitlements.

This lets us produce error messages that guide the user to add the right
entitlement to the right plugin/server/etc.

* Include component names in errors and logs

* Name APM agent specifically.

Avoids confusion with the entitlements agent.

* Entitlement component names package private

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
@prdoyle
Copy link
Contributor Author

prdoyle commented Feb 6, 2025

@prdoyle prdoyle deleted the policymanager-error-reporting branch February 6, 2025 13:16
elasticsearchmachine pushed a commit that referenced this pull request Feb 6, 2025
* Report componentName in ModuleEntitlements.

This lets us produce error messages that guide the user to add the right
entitlement to the right plugin/server/etc.

* Include component names in errors and logs

* Name APM agent specifically.

Avoids confusion with the entitlements agent.

* Entitlement component names package private

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Feb 6, 2025
* Report componentName in ModuleEntitlements.

This lets us produce error messages that guide the user to add the right
entitlement to the right plugin/server/etc.

* Include component names in errors and logs

* Name APM agent specifically.

Avoids confusion with the entitlements agent.

* Entitlement component names package private

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Feb 6, 2025
* Report componentName in ModuleEntitlements.

This lets us produce error messages that guide the user to add the right
entitlement to the right plugin/server/etc.

* Include component names in errors and logs

* Name APM agent specifically.

Avoids confusion with the entitlements agent.

* Entitlement component names package private

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants